Skip to content

Emit Self in generated code for clippy::use_self compliance#15

Merged
iainmcgin merged 6 commits intoanthropics:mainfrom
adamnemecek:main
Mar 30, 2026
Merged

Emit Self in generated code for clippy::use_self compliance#15
iainmcgin merged 6 commits intoanthropics:mainfrom
adamnemecek:main

Conversation

@adamnemecek
Copy link
Copy Markdown
Contributor

@adamnemecek adamnemecek commented Mar 27, 2026

Originally a cargo clippy --fix -- -Wclippy::use_self run on hand-written source. Expanded to update the codegen emission sites so generated output is also use_self-clean — otherwise check-generated-code CI would revert the clippy fixes on the next regeneration.

Verified by running clippy -W clippy::use_self on the googleapis stress test output (47 files, ~65K lines): zero warnings.

Codegen changes

Commit Emission site Pattern googleapis count
b1c69c1 impl_message.rs, view.rs DefaultInstance::default_instanceSelf::default() 477
b3e7faa oneof.rs impl From<T> for OneofEnumSelf::Variant(...), Self::Some(...) 41
79e2a19 message.rs Self-referential struct fields → Vec<Self> / MessageField<Self> 2
824942f view.rs Self-referential view fields → RepeatedView<'a, Self> 2

Design note: FieldInfo.struct_field_type

Self-reference detection for struct fields compares the field's type_name (proto FQN) against the current message's FQN — no token inspection needed. The result is stored as a separate FieldInfo.struct_field_type alongside the concrete rust_type, because rust_type is also consumed by serde-deserialize codegen running inside impl Visitor for _V, where Self would bind to _V instead of the message (caught by the conformance build).

Edge cases validated

  • google.api.expr.v1alpha1.Type — self-ref inside a oneof. Variant declaration correctly uses super::Type; From impls correctly use Self::Type and Self::Some. Three Self scopes, all correct.
  • TestAllRequiredTypesProto2.recursive_message (conformance) — JSON-deserialize Visitor scoping.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 27, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@adamnemecek
Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Mar 27, 2026
adamnemecek and others added 3 commits March 30, 2026 17:55
The clippy auto-fix in the original commit touched generated .rs files
directly, which check-generated-code CI would revert. This updates the
two quote! emission sites (impl_message.rs, view.rs) so codegen
actually produces Self::default() in DefaultInstance/DefaultViewInstance
impls, then regenerates.

A few of clippy's other generated-file substitutions (Vec<Self> for
self-referential struct fields, Self:: in oneof From impls) are
normalized back by regeneration — those would need separate codegen
changes and are out of scope here. The hand-written source changes
from the original commit all remain.
Covers the remaining clippy::use_self sites in generated oneof code:
  - Self::Variant(...) inside impl From<T> for OneofEnum
  - Self::Some(...) inside impl From<T> for Option<OneofEnum>

The one case left unaddressed is self-referential struct fields
(DescriptorProto.nested_type: Vec<DescriptorProto> could be Vec<Self>).
Emitting Self there requires comparing resolved field types against
the current message ident during struct generation — invasive for a
single beneficiary. Deferred.
Add FieldInfo.struct_field_type alongside rust_type. The struct-field
declaration uses struct_field_type (with Self substituted for
self-referential message types); rust_type stays concrete for the
serde-deserialize codegen, which runs inside impl Visitor for _V
where Self would bind to the wrong type.

The substitution compares field.type_name against the current
message's dotted FQN — no token-level inspection needed.

Exercised by DescriptorProto.nested_type: now Vec<Self>.

(conformance/Cargo.lock picked up buffa-descriptor from the rebase
onto main — that crate split landed in anthropics#8.)
Mirror of the owned-struct change in the previous commit, applied in
view_struct_field. Inside struct FooView<'a>, Self expands to
FooView<'a> with the lifetime applied, so RepeatedView<'a, Self> is
exactly equivalent to the concrete form.

Found by running clippy::use_self on the googleapis stress output —
the only two remaining warnings were HttpRuleView.additional_bindings
and DeclTypeView.type_params. With this commit the entire googleapis
corpus (47 files, ~65K lines) is clippy::use_self clean.
@iainmcgin iainmcgin changed the title ran argo clippy --allow-dirty --fix -- -Wclippy::use_self Emit Self in generated code for clippy::use_self compliance Mar 30, 2026
@iainmcgin
Copy link
Copy Markdown
Collaborator

Thanks for the contribution! I updated code generation to emit Self where it can do so safely, so that this won't regress with subsequent code regeneration.

@iainmcgin iainmcgin merged commit 63268e9 into anthropics:main Mar 30, 2026
7 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants